-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-4576 Allow update to supply sort option #1881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
pymongo/asynchronous/collection.py
Outdated
:return: - An instance of :class:`~pymongo.results.UpdateResult`. | ||
|
||
.. versionchanged:: 4.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4.10 -> 4.11 here and elsewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -144,6 +145,7 @@ def add_update( | |||
collation: Optional[Mapping[str, Any]] = None, | |||
array_filters: Optional[list[Mapping[str, Any]]] = None, | |||
hint: Union[str, dict[str, Any], None] = None, | |||
sort: Optional[Mapping[str, Any]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is slightly inconsistent with pymongo's other sort APIs which use _Sort
type and allow a list of key pairs as well as a mapping. I think that's okay because mapping should be the preferred approach now (after PYTHON-2878).
I say we keep it like this and only add support for a list of pairs if a user requests it. It will save us the work of adding tests for the (list or pairs) behavior.
test/utils.py
Outdated
arguments[arg_name] = list(sort_dict.items()) | ||
if spec["name"] not in ["replaceOne", "updateOne"]: | ||
sort_dict = arguments[arg_name] | ||
arguments[arg_name] = list(sort_dict.items()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this code altogether because we implemented https://jira.mongodb.org/browse/PYTHON-2878 in 4.6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -1176,8 +1188,12 @@ async def replace_one( | |||
aggregate expression context (e.g. "$$var"). | |||
:param comment: A user-provided comment to attach to this | |||
command. | |||
:param sort: Specify which document the operation updates if the query matches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a mention for these new apis in the changelog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…r into PYTHON-4576
doc/changelog.rst
Outdated
@@ -19,6 +19,8 @@ PyMongo 4.11 brings a number of changes including: | |||
until either connection succeeds or a server selection timeout error is raised. | |||
- Added :func:`repr` support to :class:`pymongo.operations.IndexModel`. | |||
- Added :func:`repr` support to :class:`pymongo.operations.SearchIndexModel`. | |||
- Added ``sort`` parameter to | |||
:meth:`~pymongo.collection.Collection.update_one` and :meth:`~pymongo.collection.Collection.replace_one` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the bulk UpdateOne/ReplaceOne operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Notes for reviewer: